-
-
Notifications
You must be signed in to change notification settings - Fork 804
feat: add math/base/special/asinf
#1950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: GUNJ JOSHI <[email protected]>
…pq.js Signed-off-by: GUNJ JOSHI <[email protected]>
Signed-off-by: GUNJ JOSHI <[email protected]>
Signed-off-by: GUNJ JOSHI <[email protected]>
Signed-off-by: GUNJ JOSHI <[email protected]>
Signed-off-by: GUNJ JOSHI <[email protected]>
Signed-off-by: GUNJ JOSHI <[email protected]>
Signed-off-by: GUNJ JOSHI <[email protected]>
Signed-off-by: GUNJ JOSHI <[email protected]>
This PR can be reviewed now. |
…oly.js Signed-off-by: Philipp Burckhardt <[email protected]>
lib/node_modules/@stdlib/math/base/special/asinf/scripts/evalpoly.js
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/math/base/special/asinf/test/fixtures/julia/runner.jl
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left two comments; otherwise this looks all good and should be ready to merge soon! Thank you!
Co-authored-by: Athan <[email protected]> Signed-off-by: GUNJ JOSHI <[email protected]>
Co-authored-by: Athan <[email protected]> Signed-off-by: GUNJ JOSHI <[email protected]>
lib/node_modules/@stdlib/math/base/special/asinf/test/test.native.js
Outdated
Show resolved
Hide resolved
Made changes suggested by you @kgryte. Along with that, modified the JS implementation. Also, what changes do we need to make in |
@gunjjoshi I've updated |
Hey @kgryte, while incorporating changes, after the latest merge, the file |
@gunjjoshi Looks like something went awry with your merge, as now changes unrelated to this PR are showing up on GitHub. You may be best to just open up a fresh PR with the proposed changes to |
Did you do an explicit |
To merge the latest changes, I did a Sure, I'll make a fresh PR for this. |
From your commit history, I am not seeing that you pushed up the merge commit. https://github.com/stdlib-js/stdlib/pull/1950/commits |
Yes @kgryte, I only committed those files which I changed, and not all of them. |
Okay. When you merge locally, you still need to push up to your remote fork on the branch you are working on the merge commit. That merge commit is what's missing in your Git history for this branch. |
Can you provide the sequence of bash commands you used when merging and pushing to your remote? |
Okay, getting it now. The sequence of commands I used were :
|
Yeah, you want to first commit any changes files, then merge, resolve any potential conflicts, commit the merge, and then push. |
Regardless, you should be able to simply copy over the changed files to a new branch, push that up, and open a fresh PR. |
Understood it now. Thanks for sharing. |
Resolves #1944.
Description
This pull request:
math/base/special/asinf
, which computes the arcsine of a single precision floating point number.Related Issues
This pull request:
math/base/special/asinf
#1944.Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers